feat: add link click conversion tracking for external links#1166
Merged
Conversation
WalkthroughAdds client-side beacon-style click tracking for external/special links in popups and a new server-side LinkClickTracking service that records and increments site-wide and per-popup link click counts. (48 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PopupJS as Popup (JS)
participant ClickH as Click Handler
participant Beacon as Analytics Beacon
participant Server as WP Server
participant DB as Database
User->>PopupJS: Click external/special link
PopupJS->>PopupJS: shouldTrackClick(url)?
PopupJS->>PopupJS: getLinkType(url)
PopupJS->>ClickH: attachClickTracking($link)
ClickH->>ClickH: validate analytics availability
ClickH->>ClickH: build payload (pid, link type, href)
ClickH->>Beacon: fire beacon POST
Beacon->>Server: deliver tracking event
Server->>Server: LinkClickTracking::track_link_click(popup_id, args)
Server->>Server: validate popup and event type
Server->>DB: atomic increment site count
Server->>DB: atomic increment popup meta count
Server->>Server: do_action('popup_maker/link_click_tracked', ...)
Server-->>Beacon: response OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
danieliser
force-pushed
the
fix/pid-mailto-tel
branch
from
0822616 to
66a5cdf
Compare
Member
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@assets/js/src/site/plugins/pum-url-tracking.js`:
- Around line 129-179: In attachClickTracking, before building or sending the
beacon payload (the data.eventData.url field), detect and redact special-scheme
targets (mailto:, tel:, javascript:) so PII/inline JS is not sent; only include
the full href for http/https links and replace special schemes with a safe
placeholder (e.g. the scheme name or null) or omit the url property. Apply the
redaction immediately after computing href/getLinkType and before applying
window.PUM.hooks and before calling window.PUM_Analytics.beacon, ensuring
getLinkType still receives the original href if needed or update calls to use
the redacted value for analytics only. Ensure this change respects existing hook
usage (pass redacted data into applyFilters) and preserves the pum-click-tracked
guard and event binding in attachClickTracking.
- Around line 84-102: The shouldTrackClick function currently counts hash-only
and same-page anchor links as conversions; update shouldTrackClick to return
false for URLs that are purely fragment identifiers (e.g., start with '#') or
point to an anchor on the same page (i.e., after parsing the URL the pathname
and host match the current location and only a hash differs). Implement a check
in shouldTrackClick to detect url.charAt(0) === '#' (or equivalent fragment-only
detection) and to parse full URLs to ignore same-page anchors, then return false
for those cases before the existing CTA check.
In `@classes/Services/LinkClickTracking.php`:
- Around line 99-108: The public action hook string used in LinkClickTracking
(the do_action call currently using 'popup_maker/link_click_tracked') must be
renamed to use the pum_ prefix; update the do_action call in the
LinkClickTracking class to emit 'pum_link_click_tracked' (or the agreed pum_
prefixed name) and update any listeners/tests referencing
'popup_maker/link_click_tracked' to the new 'pum_link_click_tracked' hook so
public API naming is consistent.
🧹 Nitpick comments (1)
classes/Services/LinkClickTracking.php (1)
119-145: Use%iplaceholders for table identifiers in prepared queries.WordPress 6.2.0+ supports
%ifor safely escaping table and column names. Since this plugin requires WordPress 6.6+, you can use%ifor both$wpdb->optionsand$wpdb->postmetatable references instead of direct interpolation.♻️ Proposed changes
- $wpdb->prepare( - "SELECT option_id FROM {$wpdb->options} WHERE option_name = %s LIMIT 1", - self::SITE_COUNT_KEY - ) + $wpdb->prepare( + 'SELECT option_id FROM %i WHERE option_name = %s LIMIT 1', + $wpdb->options, + self::SITE_COUNT_KEY + ) @@ - $wpdb->prepare( - "UPDATE {$wpdb->options} SET option_value = option_value + 1 WHERE option_name = %s", - self::SITE_COUNT_KEY - ) + $wpdb->prepare( + 'UPDATE %i SET option_value = option_value + 1 WHERE option_name = %s', + $wpdb->options, + self::SITE_COUNT_KEY + ) @@ - $wpdb->prepare( - "SELECT meta_id FROM {$wpdb->postmeta} WHERE post_id = %d AND meta_key = %s LIMIT 1", - $popup_id, - self::POPUP_META_KEY - ) + $wpdb->prepare( + 'SELECT meta_id FROM %i WHERE post_id = %d AND meta_key = %s LIMIT 1', + $wpdb->postmeta, + $popup_id, + self::POPUP_META_KEY + ) @@ - $wpdb->prepare( - "UPDATE {$wpdb->postmeta} SET meta_value = meta_value + 1 WHERE post_id = %d AND meta_key = %s", - $popup_id, - self::POPUP_META_KEY - ) + $wpdb->prepare( + 'UPDATE %i SET meta_value = meta_value + 1 WHERE post_id = %d AND meta_key = %s', + $wpdb->postmeta, + $popup_id, + self::POPUP_META_KEY + )Also applies to lines 157–179.
3 tasks
danieliser
force-pushed
the
fix/pid-mailto-tel
branch
2 times, most recently
from
960c77a to
7e4cf39
Compare
Non-HTTP protocols like mailto:, tel:, and javascript: cannot handle query parameters. The ?pid= was being appended incorrectly, breaking these special links. Added regex check to skip any URL with a protocol that isn't http(s).
- Fire beacon analytics for mailto, tel, and external link clicks - Track link clicks as conversions with eventData.type: 'link_click' - Add LinkClickTracking service with atomic counter updates - Preserve existing PID tracking for internal links only
danieliser
force-pushed
the
fix/pid-mailto-tel
branch
from
7e4cf39 to
b96c297
Compare
Modernizes SQL queries to use wpdb::prepare() %i placeholder for table name identifiers instead of PHP string interpolation.
Ensures template_redirect fires before other plugins that might intercept/redirect, preventing missed tracking events.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
5-26: Fix markdownlint MD036 by using headings for section labels.The lint error indicates emphasis is used as a heading. Converting these labels to headings will satisfy MD036 and keep formatting consistent.
♻️ Proposed change
-**Features** +### Features @@ -**Improvements** +### Improvements @@ -**Fixes** +### Fixes
♻️ Duplicate comments (1)
classes/Services/LinkClickTracking.php (1)
99-107: Rename public hook to thepum_prefix.This should follow the public hook naming convention for Popup Maker.
♻️ Proposed change
-do_action( 'popup_maker/link_click_tracked', $popup_id, $event_data ); +do_action( 'pum_link_click_tracked', $popup_id, $event_data );As per coding guidelines, public hooks should use the
pum_prefix.
This was referenced Jan 23, 2026
16 tasks
6 tasks
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
?pid=parameters appendedeventData.type: 'link_click'for segmentationChanges
JavaScript (
pum-url-tracking.js):isInternalUrl()to exclude non-HTTP protocols from PID trackingshouldTrackClick(),getLinkType(),attachClickTracking()methodsPHP (new
LinkClickTracking.phpservice):pum_analytics_conversionactionFormConversionTrackingpatternLink Behavior
/page,https://same-site.com)https://other-site.com)mailto:x@y.com)tel:123-456)javascript:fn())#,#section)Test Plan
?pid=XappendedSummary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.